Add missing licenses, and add a check #3720
Conversation
github-actions
bot
added
p: webview_flutter
|
@Hixie I added license blocks to pigeon-generated files since they didn't have a set naming scheme. I assume that's harmless to do (similar to putting it on files created by |
| // ensure that any new additions are flagged for added scrutiny in review. | ||
| // ----- | ||
| // Third-party code used in url_launcher_web. | ||
| final RegExp _workivaLicenseRegex = RegExp( |
There was a problem hiding this comment.
We should enforce that all code that doesn't have our copyright notice is in a third_party directory.
| // All Flutter-authored code. | ||
| final RegExp _bsdLicenseRegex = RegExp( | ||
| r'^(?://|#) Use of this source code is governed by a BSD-style license', | ||
| multiLine: true); |
There was a problem hiding this comment.
Can we use a static string rather than a pattern, to match our license block?
There was a problem hiding this comment.
We could, but it would mean duplicating it for bash vs. other files. What's the issue with the pattern?
(I can put the full text in the pattern, if that's the goal.)
There was a problem hiding this comment.
Seems like it would be simpler, but I see what you're saying.
FWIW this is what the flutter/flutter repo does: https://github.com/flutter/flutter/blob/master/dev/bots/analyze.dart#L226-L271
It does seem shorter than what we have here and handles bash etc.
Putting the full text in the pattern would be good regardless.
There was a problem hiding this comment.
Made it stricter as a starting point; it checks the full text of both lines, without any whitespace variation allowed.
There was a problem hiding this comment.
Since we have a bunch of variants (four different author phrasings, different years, a handful with commas after the year) I think the static string version would be too messy right now.
This PR did inspire me to continue my planned series of PRs to standardize this whole repo though (derailed by not having sign-off from repo owners to add AUTHORS to all plugins), so I'll pick that back up after this lands, and tighten this down as I go until it's at the point where we can use a string.
There was a problem hiding this comment.
I updated to use a string (using similar code to the flutter/flutter code linked above) for the license part, but not yet the copyright part. I'll switch copyright over once I've standardized.
There was a problem hiding this comment.
I took a quick look at all the licenses, and they seem to be atop of their corresponding files. Made a few comments below. I think the only missing licenses in the repo now are the example/web/index.html files, but it seems those can be corrected later very easily.
...video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/Messages.java
Show resolved
Hide resolved
|
(Also how come it's not running in this PR? Shouldn't this run pre-submit?) |
It is, it's just part of the existing "format" task. I didn't want all the overhead of another task for something that's ~instant to run, so combined it. It seemed closely linked enough to format checks conceptually to add there. |
The author and year in this repo are not yet standardized. I had to abandon an initial effort to fix that a while ago, but plan to pick it up again after this. |
* master: (49 commits) Prep for alignment with Flutter analysis options (flutter#3703) [google_maps_flutter_platform_interface] Mark constructors as const for ids (flutter#3718) [image_picker] Endorse image_picker_for_web (flutter#3717) Add missing licenses, and add a check (flutter#3720) [ci] Add libgcrypt to Docker image. (flutter#3711) Reorder the checkboxes in the PR template (flutter#3666) Re-endorse connectivity_for_web (flutter#3708) Fix missing declaration of windows' default_package (flutter#3705) Typos in comments (flutter#2846) Skip flutter upgrade for pod linting Cirrus task (flutter#3700) [cross_file] Delete. (flutter#3698) [tool] Improve check version ci so that it enforces the version in CHANGELOG and pubspec matches. (flutter#3678) Streamline CI setup, and reenable macOS credits (flutter#3697) [video_player] fixed misleading size and aspect ratio documentation (flutter#3668) [image_picker] Implemented 2860 and added Unit Test to test functionality (flutter#3685) [shared_preferences] Fix concurrent modification of the shared preferences on Android (flutter#3684) [extension_google_sign_in_as_googleapis_auth] Deleted. (flutter#3694) Skip pod lint tests (flutter#3692) [Video_Player] Remove the deprecated API reference. (flutter#3669) [google_sign_in] fix test(flutter#3690) ...
Adds a new CI check that all code files have a copyright+license block (and that it's one we are expecting to see).
Fixes the ~350 files (!) that did not have them. This includes all of the files in the .../example/ directories, following the example of flutter/flutter. (This does mean some manual intervention will be needed when generating new example directories in the future, but it's one-time per example.)
Also standardized some variants that used different line breaks than most of the rest of the repo (likely added since I standardized them all a while ago, but didn't add a check for at the time to enforce going forward), to simplify the checks.
Fixes flutter/flutter#77114
Pre-launch Checklist
[shared_preferences]///).